Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libgit2: refactor tests to use managed and unmanaged transport cleanly #777

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

aryan9600
Copy link
Member

@aryan9600 aryan9600 commented Jun 9, 2022

Refactors libgit2 checkout tests to test managed and unmanaged
transport by making sure the tests requiring unmanaged transport are run
before, any tests that require managed transport (since disabling
managed transport isn't possible). This is done via arranging the tests
carefully in alphabetically sorted names, i.e. the tests with unmanaged
transport go in checkout_test.go, which forces golang to run the tests
in that file before any other tests.

Follow-up to #727, related to #745 and branched off from #746 (to avoid increasing complexity and the size of the PR)

Signed-off-by: Sanskar Jaiswal jaiswalsanskar078@gmail.com

@aryan9600 aryan9600 requested review from pjbgf and darkowlzz and removed request for pjbgf June 9, 2022 08:55
@darkowlzz darkowlzz added area/git Git related issues and pull requests area/testing Testing related issues and pull requests labels Jun 9, 2022
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me.
Once the CI passes, we can merge this.
Thanks.

@darkowlzz
Copy link
Contributor

Could you also rebase? The branch is out-of-date.

Refactors libgit2 checkout tests to test managed and unmanaged
transport by making sure the tests requiring unmanaged transport are run
before, any tests that require managed transport (since disabling
managed transport isn't possible). This is done via arranging the tests
carefully in alphabetically sorted names, i.e. the tests with unmanaged
transport go in `checkout_test.go`, which forces golang to run the tests
in that file before any other tests.

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
@pjbgf
Copy link
Member

pjbgf commented Jun 9, 2022

It would be good to check the Managed Transport state to confirm that each test is being assessed against the correct state (initialised or not).

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @aryan9600!

LGTM

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@darkowlzz darkowlzz merged commit f6a389c into fluxcd:main Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests area/testing Testing related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants